Conversation
…cation - Add global TooltipProvider wrapper in app.tsx for entire application - Remove 36 duplicate TooltipProvider instances across 20 UI component files - Clean up imports by removing TooltipProvider from component imports - Follow Radix UI best practices for TooltipProvider placement - Reduce code by 62 lines while maintaining all tooltip functionality Closes #694 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughConsolidated TooltipProvider to the application root ( Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the application's tooltip implementation by consolidating the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Shironex
added
Refactor
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does a great job of centralizing the TooltipProvider at the application root, which is a best practice that improves maintainability and performance. However, there are a couple of important points to address. First, there's a critical issue in apps/ui/src/components/views/graph-view/components/task-node.tsx where TooltipProvider is removed from imports but still used in the component, which will break the build. Second, the new global TooltipProvider doesn't specify a delayDuration, so it will use the default of 700ms. Many of the removed providers had a delayDuration of 0ms or 200ms. This will change the tooltip behavior across the app, making them appear slower. The pull request description incorrectly states that all existing tooltip behavior is preserved. I've added specific comments with suggestions to address these points.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@apps/ui/src/components/views/graph-view/components/graph-filter-controls.tsx`:
- Around line 147-166: The Tooltip and Popover are nested causing focus/open
conflicts; fix by either adding onOpenAutoFocus={(e) => e.preventDefault()} to
the PopoverContent used with Popover/PopoverTrigger (so the popover won't
auto-focus and trigger the Tooltip) or refactor to avoid nesting Tooltip +
PopoverTrigger by wrapping the Button with the existing TooltipWrapper pattern
(use TooltipTrigger/TooltipContent only around the Button, and keep
PopoverTrigger separate) to match other components and eliminate the focus/event
interference (look for Popover, PopoverTrigger, PopoverContent, Tooltip,
TooltipTrigger, TooltipContent, and TooltipWrapper in this file).
In `@apps/ui/src/components/views/graph-view/components/task-node.tsx`:
- Line 29: The file still references TooltipProvider but no longer imports it,
causing build failure; open the TaskNode component (task-node.tsx) and either
re-add the TooltipProvider import from '@/components/ui/tooltip'
or—preferably—remove any local TooltipProvider wrapper usage around Tooltip,
TooltipTrigger, TooltipContent (lines around 29 and the block around 289-333) so
the app-level TooltipProvider is used; ensure only Tooltip, TooltipTrigger,
TooltipContent remain imported and used.
In `@apps/ui/src/components/views/settings-view/account/account-section.tsx`:
- Line 11: The JSX still wraps parts of AccountSection with TooltipProvider
(referenced as TooltipProvider) but the import was removed; either re-add the
TooltipProvider import or—preferably—remove the local <TooltipProvider> wrapper
in account-section.tsx so the global provider is used; locate the
TooltipProvider usage around the Tooltip, TooltipTrigger, TooltipContent blocks
in the AccountSection component and delete the wrapper tags (or restore the
import if you intend a local provider).
apps/ui/src/components/views/graph-view/components/graph-filter-controls.tsx
Show resolved
Hide resolved
- Add delayDuration={300} to global TooltipProvider in app.tsx to
maintain consistent tooltip timing (previously many components used
delayDuration={200}, so 300ms is a good compromise per review)
- Remove leftover TooltipProvider wrappers in task-node.tsx that were
still referenced after import was removed (causing build failure)
- Remove leftover TooltipProvider wrapper in account-section.tsx
- Fix Tooltip+Popover nesting focus management issue in
graph-filter-controls.tsx by adding onOpenAutoFocus={(e) =>
e.preventDefault()} to PopoverContent components
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/ui/src/components/views/graph-view/components/graph-filter-controls.tsx`:
- Around line 176-212: The row click plus Checkbox.onCheckedChange both fire and
can toggle twice; to fix, stop the pointer event from reaching the row by
preventing propagation on the Checkbox elements used in the four places: add a
pointer event handler that calls e.stopPropagation() (e.g. onPointerDown={(e) =>
e.stopPropagation()}) to each Checkbox in the Select All rows and in the
per-category and per-status rows (the Checkbox inside the JSX that references
selectedCategories/availableCategories and selectedStatuses/availableStatuses),
leaving the row onClick handlers (handleSelectAllCategories,
handleCategoryToggle, handleSelectAllStatuses, handleStatusToggle) intact so
clicks on the row still work but direct clicks on the Checkbox do not bubble.
🧹 Nitpick comments (1)
apps/ui/src/components/views/settings-view/account/account-section.tsx (1)
11-11: Consider importing Tooltip components from the components barrel (if available).If
Tooltip,TooltipTrigger, andTooltipContentare exported via the app’scomponentsbarrel, prefer that to a deep path to keep intra-app imports stable. Based on learnings, ...
| <div | ||
| className="flex items-center gap-2 px-2 py-1.5 rounded hover:bg-accent cursor-pointer" | ||
| onClick={handleSelectAllCategories} | ||
| > | ||
| <Checkbox | ||
| checked={ | ||
| selectedCategories.length === availableCategories.length && | ||
| availableCategories.length > 0 | ||
| } | ||
| onCheckedChange={handleSelectAllCategories} | ||
| /> | ||
| <span className="text-sm font-medium"> | ||
| {selectedCategories.length === availableCategories.length | ||
| ? 'Deselect All' | ||
| : 'Select All'} | ||
| </span> | ||
| </div> | ||
| </PopoverContent> | ||
| </Popover> | ||
|
|
||
| {/* Status Filter Dropdown */} | ||
| <Popover> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <PopoverTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className={cn( | ||
| 'h-8 px-2 gap-1.5', | ||
| selectedStatuses.length > 0 && 'bg-brand-500/20 text-brand-500' | ||
| )} | ||
| > | ||
| <CircleDot className="w-4 h-4" /> | ||
| <span className="text-xs max-w-[120px] truncate">{statusButtonLabel}</span> | ||
| <ChevronDown className="w-3 h-3 opacity-50" /> | ||
| </Button> | ||
| </PopoverTrigger> | ||
| </TooltipTrigger> | ||
| <TooltipContent>Filter by Status</TooltipContent> | ||
| </Tooltip> | ||
| <PopoverContent align="start" className="w-56 p-2"> | ||
| <div className="space-y-2"> | ||
| <div className="text-xs font-medium text-muted-foreground px-2 py-1">Status</div> | ||
|
|
||
| {/* Select All option */} | ||
| <div | ||
| className="flex items-center gap-2 px-2 py-1.5 rounded hover:bg-accent cursor-pointer" | ||
| onClick={handleSelectAllStatuses} | ||
| > | ||
| <Checkbox | ||
| checked={selectedStatuses.length === STATUS_FILTER_OPTIONS.length} | ||
| onCheckedChange={handleSelectAllStatuses} | ||
| /> | ||
| <span className="text-sm font-medium"> | ||
| {selectedStatuses.length === STATUS_FILTER_OPTIONS.length | ||
| ? 'Deselect All' | ||
| : 'Select All'} | ||
| </span> | ||
| </div> | ||
| <div className="h-px bg-border" /> | ||
|
|
||
| <div className="h-px bg-border" /> | ||
|
|
||
| {/* Status list */} | ||
| <div className="space-y-0.5"> | ||
| {STATUS_FILTER_OPTIONS.map((status) => { | ||
| const config = statusDisplayConfig[status]; | ||
| const StatusIcon = config.icon; | ||
| return ( | ||
| <div | ||
| key={status} | ||
| className="flex items-center gap-2 px-2 py-1.5 rounded hover:bg-accent cursor-pointer" | ||
| onClick={() => handleStatusToggle(status)} | ||
| > | ||
| <Checkbox | ||
| checked={selectedStatuses.includes(status)} | ||
| onCheckedChange={() => handleStatusToggle(status)} | ||
| /> | ||
| <StatusIcon className={cn('w-3.5 h-3.5', config.colorClass)} /> | ||
| <span className="text-sm">{config.label}</span> | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| {/* Category list */} | ||
| <div className="max-h-48 overflow-y-auto space-y-0.5"> | ||
| {availableCategories.length === 0 ? ( | ||
| <div className="text-xs text-muted-foreground px-2 py-2"> | ||
| No categories available | ||
| </div> | ||
| ) : ( | ||
| availableCategories.map((category) => ( | ||
| <div | ||
| key={category} | ||
| className="flex items-center gap-2 px-2 py-1.5 rounded hover:bg-accent cursor-pointer" | ||
| onClick={() => handleCategoryToggle(category)} | ||
| > | ||
| <Checkbox | ||
| checked={selectedCategories.includes(category)} | ||
| onCheckedChange={() => handleCategoryToggle(category)} | ||
| /> |
There was a problem hiding this comment.
Avoid double‑toggling on checkbox rows.
On Line 176–186 and Line 204–212 (categories) and Line 252–259 and Line 275–283 (statuses), the row onClick plus Checkbox.onCheckedChange can both fire on pointer clicks, toggling twice and canceling the change. Consider stopping propagation from the Checkbox (or removing one handler) for both sections.
🔧 One minimal fix (apply to all four checkbox rows)
- <Checkbox
+ <Checkbox
checked={
selectedCategories.length === availableCategories.length &&
availableCategories.length > 0
}
onCheckedChange={handleSelectAllCategories}
+ onClick={(e) => e.stopPropagation()}
/>- <Checkbox
+ <Checkbox
checked={selectedCategories.includes(category)}
onCheckedChange={() => handleCategoryToggle(category)}
+ onClick={(e) => e.stopPropagation()}
/>- <Checkbox
+ <Checkbox
checked={selectedStatuses.length === STATUS_FILTER_OPTIONS.length}
onCheckedChange={handleSelectAllStatuses}
+ onClick={(e) => e.stopPropagation()}
/>- <Checkbox
+ <Checkbox
checked={selectedStatuses.includes(status)}
onCheckedChange={() => handleStatusToggle(status)}
+ onClick={(e) => e.stopPropagation()}
/>Also applies to: 252-283
🤖 Prompt for AI Agents
In `@apps/ui/src/components/views/graph-view/components/graph-filter-controls.tsx`
around lines 176 - 212, The row click plus Checkbox.onCheckedChange both fire
and can toggle twice; to fix, stop the pointer event from reaching the row by
preventing propagation on the Checkbox elements used in the four places: add a
pointer event handler that calls e.stopPropagation() (e.g. onPointerDown={(e) =>
e.stopPropagation()}) to each Checkbox in the Select All rows and in the
per-category and per-status rows (the Checkbox inside the JSX that references
selectedCategories/availableCategories and selectedStatuses/availableStatuses),
leaving the row onClick handlers (handleSelectAllCategories,
handleCategoryToggle, handleSelectAllStatuses, handleStatusToggle) intact so
clicks on the row still work but direct clicks on the Checkbox do not bubble.
1 participant
Summary
This PR implements the fix for GitHub Issue #694 by creating a single global
TooltipProviderin the application root and removing all duplicate instances throughout the codebase.Changes Implemented
apps/ui/src/app.tsxwrapping the entire application (RouterProvider and SplashScreen)Files Modified
Core Application
apps/ui/src/app.tsx- Added global TooltipProvider wrapperSidebar Components (3 files)
apps/ui/src/components/layout/sidebar/components/sidebar-navigation.tsxapps/ui/src/components/layout/sidebar/components/sidebar-header.tsxapps/ui/src/components/layout/sidebar/components/sidebar-footer.tsxBoard View Components (9 files)
apps/ui/src/components/views/board-view/board-controls.tsxapps/ui/src/components/views/board-view/kanban-board.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsxapps/ui/src/components/views/board-view/components/kanban-card/card-badges.tsxapps/ui/src/components/views/board-view/components/list-view/list-row.tsxWorktree Panel Components (4 files)
apps/ui/src/components/views/board-view/worktree-panel/components/tooltip-wrapper.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-dropdown-item.tsxGraph View Components (3 files)
apps/ui/src/components/views/graph-view/components/task-node.tsxapps/ui/src/components/views/graph-view/components/graph-filter-controls.tsxapps/ui/src/components/views/graph-view/components/graph-controls.tsxOther Components (2 files)
apps/ui/src/components/ui/keyboard-map.tsxapps/ui/src/components/views/settings-view/account/account-section.tsxBenefits
Testing
All tooltips across the application should continue to work as expected:
Closes #694
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.